-
-
Notifications
You must be signed in to change notification settings - Fork 247
Use Eldev #669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Eldev #669
Conversation
p4v4n
commented
Nov 4, 2023
•
edited
Loading
edited
- For Use Eldev #666
Awesome! I take it that you're following the example from clj-refactor or cider? |
Yes. Mostly copied from both the projects with some minor changes. |
@@ -46,7 +46,7 @@ | |||
(bb-edn-src (expand-file-name "src" temp-dir))) | |||
(write-region "{}" nil bb-edn) | |||
(make-directory bb-edn-src) | |||
(expect (clojure-project-dir bb-edn-src) | |||
(expect (expand-file-name (clojure-project-dir bb-edn-src)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this is needed. This test was failing only on windows.
I don't have a windows machine to reproduce/fix it properly.
@@ -3129,7 +3129,7 @@ Assumes cursor is at beginning of function." | |||
"Add an arity to a function. | |||
|
|||
Assumes cursor is at beginning of function." | |||
(let ((beg-line (what-line)) | |||
(let ((beg-line (line-number-at-pos)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this change 3 tests for clojure-add-arity
were failing with an additional \n in the generated output. Only happens when running tests with eldev.(Might be because what-line
is an interactive fn with side effects)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounding good. Please confirm that you've run this modified version locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have run it locally and it works fine.
Locally both the versions work but not using what-line
is still better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - much appreciated work 🙌
.circleci/config.yml
Outdated
- test-ubuntu-emacs-29 | ||
- test-ubuntu-emacs-master | ||
- test-windows-emacs-latest | ||
- test-macos-emacs-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- test-macos-emacs-latest | |
- test-macos-emacs-latest: | |
requires: | |
- test-ubuntu-emacs-29 |
macOS builds are little more precious
@@ -3129,7 +3129,7 @@ Assumes cursor is at beginning of function." | |||
"Add an arity to a function. | |||
|
|||
Assumes cursor is at beginning of function." | |||
(let ((beg-line (what-line)) | |||
(let ((beg-line (line-number-at-pos)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounding good. Please confirm that you've run this modified version locally
@@ -3141,7 +3141,7 @@ Assumes cursor is at beginning of function." | |||
(insert "[") | |||
(save-excursion (insert "])\n("))) | |||
((looking-back "\\[" 1) ;; single-arity fn | |||
(let* ((same-line (string= beg-line (what-line))) | |||
(let* ((same-line (= beg-line (line-number-at-pos))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please confirm that you've run this modified version locally
Awesome. If you'd wish to do more contributions for clojure-mode and friends, don't hesitate to ping us in our many GH issues, or #cider. |